Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit tests #274

Closed
wants to merge 1 commit into from
Closed

Unit tests #274

wants to merge 1 commit into from

Conversation

makcandrov
Copy link
Contributor

  • MarketLib
  • UtilsLib
  • ? FixedPointMathLib
  • SharesMathLib

Should we also test SafeTransferLib (which is already extensively tested here) and the WAD based operations from FixedPointMathLib (which are already extensively tested here)?

@makcandrov makcandrov changed the base branch from main to feat/tests August 11, 2023 14:24
Jean-Grimal
Jean-Grimal previously approved these changes Aug 11, 2023
@pakim249CAL
Copy link
Contributor

pakim249CAL commented Aug 13, 2023

It seems that many libraries are being redone, including safetransfer. so I would scope this PR to specific libraries that are stable, and create issues to track each library that we plan to test. Perhaps maybe even splitting this PR to one PR for each lib.

#280

In fact, I would generally advise that each branching of testing come with its own issue and PR. Big PRs are not ze way IMO, especially for tests that can be cleanly separated from each other since they should not depend on each other (unless the base tests are being worked on)

Base automatically changed from feat/tests to main August 15, 2023 19:54
@MerlinEgalite MerlinEgalite dismissed Jean-Grimal’s stale review August 15, 2023 19:54

The base branch was changed.

@makcandrov
Copy link
Contributor Author

I would scope this PR to specific libraries that are stable

I'll split the PR then!

@makcandrov makcandrov closed this Aug 16, 2023
@makcandrov makcandrov deleted the feat/unit-tests branch August 16, 2023 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants